src: use custom fprintf alike to write errors to stderr #31446

addaleax · 2020-01-21T19:53:14Z

src: add C++-style sprintf utility

Add an utility that handles C++-style strings and objects well.

src: use custom fprintf alike to write errors to stderr

This allows printing errors that contain nul characters, for example.

Fixes: #28761
Fixes: #31218

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
commit message follows commit guidelines

Add an utility that handles C++-style strings and objects well.

This allows printing errors that contain nul characters, for example. Fixes: nodejs#28761 Fixes: nodejs#31218

test/cctest/test_util.cc

nodejs-github-bot · 2020-01-21T19:56:13Z

CI: https://ci.nodejs.org/job/node-test-pull-request/28540/

nodejs-github-bot · 2020-01-21T20:31:07Z

CI: https://ci.nodejs.org/job/node-test-pull-request/28542/

devsnek

this is pretty cool

bnoordhuis

Pretty cool. I do wonder if this won't bloat the binary if it's used in a lot of compilation units. Each unit will get its own, possibly unrolled copy.

src/debug_utils-inl.h

From the issue: > Some servers deviate from HTTP spec enougth that Node.js can't > communicate with them, but "work" when `--insecure-http-parser` > is enabled globally. It would be useful to be able to use this > mode, as a client, only when connecting to known bad servers. This is largely equivalent to nodejs#31446 in terms of code changes. Fixes: nodejs#31440 Refs: nodejs#31446

addaleax · 2020-01-21T22:06:26Z

Pretty cool. I do wonder if this won't bloat the binary if it's used in a lot of compilation units. Each unit will get its own, possibly unrolled copy.

The binary size definitely increases with this, yes. The functions do get deduplicated as they aren’t inlined, though, at least for me (with the __attribute__((noinline)) working).

nodejs-github-bot · 2020-01-21T22:08:32Z

CI: https://ci.nodejs.org/job/node-test-pull-request/28550/

src/node_errors.cc

src/debug_utils-inl.h

src/debug_utils.cc

Co-Authored-By: Ben Noordhuis <[email protected]>

nodejs-github-bot · 2020-01-22T17:44:19Z

CI: https://ci.nodejs.org/job/node-test-pull-request/28566/

nodejs-github-bot · 2020-01-23T09:19:35Z

CI: https://ci.nodejs.org/job/node-test-pull-request/28589/

nodejs-github-bot · 2020-01-23T14:33:21Z

CI: https://ci.nodejs.org/job/node-test-pull-request/28592/

Add an utility that handles C++-style strings and objects well. PR-URL: #31446 Fixes: #28761 Fixes: #31218 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Rich Trott <[email protected]>

This allows printing errors that contain nul characters, for example. Fixes: #28761 Fixes: #31218 PR-URL: #31446 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Rich Trott <[email protected]>

addaleax · 2020-01-23T21:43:36Z

Landed in f6c6236...32e7e81

From the issue: > Some servers deviate from HTTP spec enougth that Node.js can't > communicate with them, but "work" when `--insecure-http-parser` > is enabled globally. It would be useful to be able to use this > mode, as a client, only when connecting to known bad servers. This is largely equivalent to #31446 in terms of code changes. Fixes: #31440 Refs: #31446 PR-URL: #31448 Reviewed-By: Sam Roberts <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Rich Trott <[email protected]>

From the issue: > Some servers deviate from HTTP spec enougth that Node.js can't > communicate with them, but "work" when `--insecure-http-parser` > is enabled globally. It would be useful to be able to use this > mode, as a client, only when connecting to known bad servers. This is largely equivalent to nodejs#31446 in terms of code changes. Fixes: nodejs#31440 Refs: nodejs#31446 PR-URL: nodejs#31448 Reviewed-By: Sam Roberts <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Rich Trott <[email protected]>

Backport 7fc5656 Original commit message: From the issue: > Some servers deviate from HTTP spec enougth that Node.js can't > communicate with them, but "work" when `--insecure-http-parser` > is enabled globally. It would be useful to be able to use this > mode, as a client, only when connecting to known bad servers. This is largely equivalent to nodejs/node#31446 in terms of code changes. Fixes: nodejs/node#31440 Refs: nodejs/node#31446 Backport-PR-URL: nodejs/node#31500 PR-URL: nodejs/node#31448 Reviewed-By: Sam Roberts <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Rich Trott <[email protected]>

Add an utility that handles C++-style strings and objects well. PR-URL: #31446 Fixes: #28761 Fixes: #31218 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Rich Trott <[email protected]>

This allows printing errors that contain nul characters, for example. Fixes: #28761 Fixes: #31218 PR-URL: #31446 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Rich Trott <[email protected]>

Add an utility that handles C++-style strings and objects well. PR-URL: #31446 Fixes: #28761 Fixes: #31218 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Rich Trott <[email protected]>

This allows printing errors that contain nul characters, for example. Fixes: #28761 Fixes: #31218 PR-URL: #31446 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Rich Trott <[email protected]>

Add an utility that handles C++-style strings and objects well. PR-URL: #31446 Fixes: #28761 Fixes: #31218 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Rich Trott <[email protected]>

This allows printing errors that contain nul characters, for example. Fixes: #28761 Fixes: #31218 PR-URL: #31446 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Rich Trott <[email protected]>

addaleax added 2 commits January 21, 2020 20:48

src: add C++-style sprintf utility

a35bf6b

Add an utility that handles C++-style strings and objects well.

src: use custom fprintf alike to write errors to stderr

ec00fda

This allows printing errors that contain nul characters, for example. Fixes: nodejs#28761 Fixes: nodejs#31218

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. labels Jan 21, 2020

addaleax commented Jan 21, 2020

View reviewed changes

test/cctest/test_util.cc Show resolved Hide resolved

jasnell approved these changes Jan 21, 2020

View reviewed changes

addaleax added 2 commits January 21, 2020 21:23

fixup! src: add C++-style sprintf utility

6db1bb7

fixup! src: use custom fprintf alike to write errors to stderr

3d90553

devsnek approved these changes Jan 21, 2020

View reviewed changes

bnoordhuis approved these changes Jan 21, 2020

View reviewed changes

src/debug_utils-inl.h Outdated Show resolved Hide resolved

addaleax mentioned this pull request Jan 21, 2020

http: make --insecure-http-parser configurable per-stream or per-server #31448

Closed

4 tasks

fixup! src: add C++-style sprintf utility

8824f8b

addaleax force-pushed the sprintf branch from 49d2ea4 to 8824f8b Compare January 21, 2020 22:06

addaleax added 2 commits January 21, 2020 23:07

fixup! src: use custom fprintf alike to write errors to stderr

bed1750

fixup! fixup! src: add C++-style sprintf utility

cb717aa

fixup! fixup! fixup! src: add C++-style sprintf utility

3a1c801

cjihrig approved these changes Jan 22, 2020

View reviewed changes

legendecas reviewed Jan 22, 2020

View reviewed changes

src/node_errors.cc Show resolved Hide resolved

bnoordhuis approved these changes Jan 22, 2020

View reviewed changes

src/debug_utils-inl.h Outdated Show resolved Hide resolved

src/debug_utils.cc Show resolved Hide resolved

addaleax and others added 2 commits January 22, 2020 17:48

fixup! src: add C++-style sprintf utility

a851023

Co-Authored-By: Ben Noordhuis <[email protected]>

fixup! src: use custom fprintf alike to write errors to stderr

bd8c9d6

addaleax added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. and removed author ready PRs that have at least one approval, no pending requests for changes, and a CI started. labels Jan 22, 2020

fixup! src: add C++-style sprintf utility

13b9da7

addaleax added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jan 22, 2020

addaleax mentioned this pull request Jan 22, 2020

quic: additional cleanups and fixups nodejs/quic#294

Closed

Trott approved these changes Jan 23, 2020

View reviewed changes

addaleax closed this Jan 23, 2020

addaleax deleted the sprintf branch January 23, 2020 21:43

addaleax mentioned this pull request Jan 24, 2020

[v12.x] http: make --insecure-http-parser configurable per-stream or per-server #31500

Closed

4 tasks

addaleax mentioned this pull request Jan 25, 2020

tls: Fix crash when internal debug enabled #31511

Closed

2 tasks

codebytere mentioned this pull request Feb 17, 2020

v13.9.0 proposal #31837

Merged

codebytere mentioned this pull request Mar 17, 2020

v12.16.2 proposal #32313

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

src: use custom fprintf alike to write errors to stderr #31446

src: use custom fprintf alike to write errors to stderr #31446

addaleax commented Jan 21, 2020

nodejs-github-bot commented Jan 21, 2020

nodejs-github-bot commented Jan 21, 2020

devsnek left a comment

bnoordhuis left a comment

addaleax commented Jan 21, 2020 •

edited

Loading

nodejs-github-bot commented Jan 21, 2020

nodejs-github-bot commented Jan 22, 2020

nodejs-github-bot commented Jan 23, 2020

nodejs-github-bot commented Jan 23, 2020

addaleax commented Jan 23, 2020

src: use custom fprintf alike to write errors to stderr #31446

src: use custom fprintf alike to write errors to stderr #31446

Conversation

addaleax commented Jan 21, 2020

src: add C++-style sprintf utility

src: use custom fprintf alike to write errors to stderr

Checklist

nodejs-github-bot commented Jan 21, 2020

nodejs-github-bot commented Jan 21, 2020

devsnek left a comment

Choose a reason for hiding this comment

bnoordhuis left a comment

Choose a reason for hiding this comment

addaleax commented Jan 21, 2020 • edited Loading

nodejs-github-bot commented Jan 21, 2020

nodejs-github-bot commented Jan 22, 2020

nodejs-github-bot commented Jan 23, 2020

nodejs-github-bot commented Jan 23, 2020

addaleax commented Jan 23, 2020

addaleax commented Jan 21, 2020 •

edited

Loading